feat(core): Add deferred segment-span transaction capture#21839
Open
andreiborza wants to merge 9 commits into
Open
feat(core): Add deferred segment-span transaction capture#21839andreiborza wants to merge 9 commits into
andreiborza wants to merge 9 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
920659d to
1397906
Compare
andreiborza
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
a1613c2 to
6bd79ac
Compare
andreiborza
requested review from
JPeer264 and
mydea
and removed request for
a team
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
29ce501 to
c741940
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
2 times, most recently
from
e005612 to
924ce11
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
c741940 to
4b3fc03
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
924ce11 to
630bb67
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
4b3fc03 to
14cb421
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
630bb67 to
15154ed
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
14cb421 to
6b8db6e
Compare
nicohrubec
reviewed
Jun 30, 2026
Comment on lines
+57
to
+62
| // Clients whose segment-span transaction capture should be deferred (rather than run synchronously on | ||
| // span end), mapped to the function that queues a deferred capture. Tracked per client rather than as | ||
| // a process-wide flag so pending captures and their timer cannot leak across `Sentry.init()` calls — | ||
| // a client that never opts in is simply absent. Enabled per client by SDKs that assemble transactions | ||
| // from the live span tree on root-span end (e.g. the Node SDK), which would otherwise drop children | ||
| // that close after it. Every other setup keeps its synchronous capture. |
Member
There was a problem hiding this comment.
Suggested change
| // Clients whose segment-span transaction capture should be deferred (rather than run synchronously on | |
| // span end), mapped to the function that queues a deferred capture. Tracked per client rather than as | |
| // a process-wide flag so pending captures and their timer cannot leak across `Sentry.init()` calls — | |
| // a client that never opts in is simply absent. Enabled per client by SDKs that assemble transactions | |
| // from the live span tree on root-span end (e.g. the Node SDK), which would otherwise drop children | |
| // that close after it. Every other setup keeps its synchronous capture. | |
| // Clients that opted into deferred segment-span capture (see `_INTERNAL_se | |
| tDeferSegmentSpanCapture`), | |
| // mapped to the function that queues a deferred capture. Keyed by client r | |
| ather than a process-wide | |
| // flag so pending captures and their timer cannot leak across `Sentry.init | |
| ()` calls. |
just a suggestion but I am wondering if we could cut this comment down a bit, reads quite lengthy 😅
Comment on lines
+65
to
+67
| // Spans already included in a captured transaction. Used so a child that ends after its root segment | ||
| // was captured can be emitted as its own orphan transaction (see `_onSpanEnded`) without any span ever | ||
| // being sent in more than one transaction. |
Member
There was a problem hiding this comment.
Suggested change
| // Spans already included in a captured transaction. Used so a child that ends after its root segment | |
| // was captured can be emitted as its own orphan transaction (see `_onSpanEnded`) without any span ever | |
| // being sent in more than one transaction. | |
| // Track spans already sent in a transaction, so we can emit spans ending after the root in a separate transaction |
| const CAPTURED_SPANS = new WeakSet<Span>(); | ||
|
|
||
| /** | ||
| * Defer a client's segment-span transaction capture. Set once by the SDK during setup (e.g. the Node |
Member
There was a problem hiding this comment.
same here, maybe we can cut this down a bit
| // fires on a later tick must reach the client active at span end and never whatever client | ||
| // is current when the timer fires (e.g. a different client after re-init), and the scope's | ||
| // client reference can be reassigned. Only the snapshot is deferred, so late children land. | ||
| client.captureEvent(transactionEvent, undefined, scope); |
Member
There was a problem hiding this comment.
q: the comment says do not resolve from the scope but then you pass the scope here, how does that work?
Member
Author
There was a problem hiding this comment.
Thanks, clarified this in the refactor.
| debouncedDrain.flush(); | ||
| }); | ||
|
|
||
| DEFERRED_SEGMENT_SPAN_CAPTURES.set(client, capture => { |
Member
There was a problem hiding this comment.
as discussed offline: maybe we can think about simplifying this by not doing this per client or using an explicit queue class or something like that. if not also fine just think this is quite hard to understand at first pass
Member
Author
There was a problem hiding this comment.
I reworked this to split it out into its own file with the benefit of reducing the browser bundle size a bit (still some hit due to having to call these but better than before where debounce et al were living in SentrySpan.
As for the need of doing this per client: This mimics what we do in logs/metrics already and it's needed because closing a client could potentially end up flushing spans that are not yet ready to be built into a transaction (for example because they are from a different client).
Overall I agree, it is a bit confusing but all of this code can be deleted once we get rid of transactions at least.
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
15154ed to
7837eb8
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
6b8db6e to
2df53ad
Compare
Member
Author
|
I'm going to rework this slightly so it has no impact on browser SDKs. |
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
51859a2 to
f25bf84
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
7837eb8 to
97e8266
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
c11b10c to
e7f6447
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
2 times, most recently
from
cfbfb8b to
1c93e34
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
6de76e6 to
f2439e8
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f2439e8. Configure here.
| const enqueue = client && CLIENT_QUEUES.get(client); | ||
| if (!enqueue) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Late orphans dropped without queue
Medium Severity
When a late child should become an orphan transaction, onChildSpanEnded returns without capturing if the current getClient() has no defer queue. Unlike onSegmentSpanEnded, there is no synchronous fallback, so those spans are dropped even though the root was already sent.
Reviewed by Cursor Bugbot for commit f2439e8. Configure here.
Add per-client deferral of the segment-span transaction capture. The transaction is otherwise assembled synchronously from the live span tree when the root span ends, dropping child spans whose instrumentation closes them after it - in the same tick (diagnostics-channel `asyncEnd`) or on a later tick (e.g. prisma engine spans). When a client opts in via `_INTERNAL_setDeferSegmentSpanCapture`, a debounced timer (the one the OpenTelemetry span exporter uses) delays the snapshot so those children land first, and drains on the client `flush` hook so `Sentry.flush()` / `close()` stays safe. The browser keeps its synchronous capture. The opt-in call is wired separately (the Node SDK enables it on the SentryTracerProvider path).
Extract the defer/orphan machinery (per-client queues, debounced drain, flush wiring, orphan detection, the CAPTURED_SPANS set) out of SentrySpan into a node-only deferSegmentSpanCapture module, registered through a carrier-based strategy seam that mirrors set/getAsyncContextStrategy. SentrySpan reads the seam and captures synchronously when none is registered, so browser bundles that never register the strategy tree-shake the machinery away.
…sion, flush draining Covers the three behaviors behind the strategy, driven through SentrySpan.end() with fake timers: a child ending before the debounce fires lands in the deferred transaction; a child ending after the snapshot is emitted as its own orphan transaction tagged sentry.parent_span_already_sent; and pending captures drain synchronously on the client's flush hook.
…pture Drops the CAPTURED_SPAN_CLIENTS routing map and the scope/client params threaded through the strategy. Each client gets one debounced queue (mirroring the OpenTelemetry span exporter's per-instance buffer); the capturing client is bound when the span ends and used at drain, so a deferred transaction always lands on the client that created the span. The strategy interface is now just the convert callback.
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
f2439e8 to
1d7f095
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the ability to defer the assembly of transactions to avoid dropping spans from transactions that end shortly after the segment span itself. Additionally, it also handles children that end after the debounce fired and transactions have already been sent. Spans that don't quite make it will end up as their own transaction in the same trace instead of being dropped .
This mimics what is already done today in the span exporter (a buffer + debounced flush).
Why
SentrySpan assembles a transaction synchronously from the span tree the instant the segment span ends. But some child spans are closed by their instrumentation after the root ends.
For example:
asyncEnd/response callback that runs after the root handler returns.@prisma/instrumentationemits its engine spans on a later tick once it receives the engine trace data.Without deferral those children aren't in the tree yet at root-end, so they're silently dropped from the transaction. With the OTel SDK, this never happened because the
SentrySpanExporteralready buffers finished spans and flushes on a debounced timer. The SentryTracerProvider has no exporter, so defer reinstates that buffering window so late-ending children land before the snapshot.This is used and tested in #21680's integration/e2e tests.